Skip to content

Backport the upstreamed Bulk Enroll API - #64

Merged
bdero merged 2 commits into
opencraft-release/ficus.3-clouderafrom
bdero/backport-bulk-enroll
Jul 23, 2017
Merged

Backport the upstreamed Bulk Enroll API#64
bdero merged 2 commits into
opencraft-release/ficus.3-clouderafrom
bdero/backport-bulk-enroll

Conversation

@bdero

@bdero bdero commented Jul 19, 2017

Copy link
Copy Markdown

bdero added 2 commits July 19, 2017 02:22
… behave with strings and lists

(cherry picked from commit 43e161f)
@bdero
bdero force-pushed the bdero/backport-bulk-enroll branch from 9cbfb4c to e3e8dbe Compare July 20, 2017 22:31
@bdero

bdero commented Jul 20, 2017

Copy link
Copy Markdown
Author

Note: I removed this fix, which is not compatible with the version of DRF used in Ficus: https://github.com/edx/edx-platform/pull/15584

See this PR for details!: https://github.com/edx/edx-platform/pull/15631

CC @pomegranited

@pomegranited

Copy link
Copy Markdown

@bdero Does it make sense to include both https://github.com/edx/edx-platform/pull/15584 and https://github.com/edx/edx-platform/pull/15631 in this change?

Since setting the request.POST parameters explicitly is non-standard, it seems like letting the JSON data come in as a POST parameter is a cleaner solution for all versions of django-rest-framework.

@bdero

bdero commented Jul 21, 2017

Copy link
Copy Markdown
Author

@pomegranited There was some pretty significant refactoring that happened between the version of DRF that Cloudera is using and edx:edx-platform/master. Since the DRF request POST parameters are not a property in the Cloudera version of DRF, I suspect that setting the content type in the underlying Django request won't result in the POST params being pulled from the data dict. (It's possible this behavior still exists through a dunder hook or similar, but I couldn't find anything relevant in the code)

@pomegranited

Copy link
Copy Markdown

@bdero No worries, thanks for your explanation.
👍

  • I tested this on the appserver deployed from this branch.
  • I read through the code
  • I checked for accessibility issues
  • Includes documentation - cherry-picked code, no extra docs required.

@bdero
bdero merged commit 7566bac into opencraft-release/ficus.3-cloudera Jul 23, 2017
@bdero

bdero commented Jul 23, 2017

Copy link
Copy Markdown
Author

@pomegranited Thank you!

@bdero
bdero deleted the bdero/backport-bulk-enroll branch July 23, 2017 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants